Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor existing implementation for swift-driver change #208

Merged
merged 3 commits into from
May 30, 2023

Conversation

polac24
Copy link
Collaborator

@polac24 polac24 commented May 30, 2023

As it is highly possible that Xcode 15 beta (expected on in a week, on Jun 5th) will no longer support SWIFT_USE_INTEGRATED_DRIVER: NO, we should be ready to add support for the integrated driver.

This is a first PR that refactors the existing implementation so it the swift-driver PR will be easier to reason about.

This PR doesn't change anything in the business logic.

Includes:

  • Adds a mode in the BuildSettingsIntegrateAppender to specify if the SWIFT_USE_INTEGRATED_DRIVER should be added or not (for now, always add)
  • Splits Swiftc into 2 phases: emit module and compilation. With swiftc, this is always done in a single process (at least from XCRemoteCache's perspective), in the driver world, that will be 2 independent processes
  • Adds extra options for the input (*.swift) and output (*.d, *.o etc) files: either via a file (used currently) or explicitly in the cmd argument list
  • XCSwiftc's logic is broken down to the XCSwiftAbstract, which will be shared with the XCSwiftFrontend
  • Making some params optional, which are nil in the swift-frontend

Next steps

Add swift-frontend support (draft PR)

@polac24 polac24 force-pushed the support-xcode14 branch from ec8ac62 to 616ac09 Compare May 30, 2023 04:24
@polac24 polac24 force-pushed the support-xcode14 branch from 616ac09 to 5749762 Compare May 30, 2023 04:25
@polac24 polac24 marked this pull request as ready for review May 30, 2023 04:46
Copy link
Contributor

@CognitiveDisson CognitiveDisson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested but the changes look good.

@polac24
Copy link
Collaborator Author

polac24 commented May 30, 2023

We have several E2E tests so hopefully that covers all cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants